Add a branch matcher to the server side repo config #1383

dghubble · 2021-02-02T01:18:27Z

Add a branch regex to the repo.yaml config to allow Atlantis to accept only pull requests with a given base branch (the branch
a PR would merge into).
branch is optional. By default its unset and Atlantis will match webhooks for pull requests for any branch (no change)

Match any PR,

repos:
  - id: /.*/
    branch: /.*/

Match only PRs with master or main base branch.

repos:
  - id: /.*/
    branch: /(main|master)/

Why?

Some repos have unique pull request branching practices. For example, plan/apply from feature branches merging into master, but separate flows for merging master into a release branch (where Atlantis isn't used). Adding a regex allows for flexible workflow options. For example, you could have Atlantis ignore "release" branches.

codecov · 2021-02-02T01:20:10Z

Codecov Report

Merging #1383 (0d35199) into master (832afea) will decrease coverage by 0.12%.
The diff coverage is 27.77%.

@@            Coverage Diff             @@
##           master    #1383      +/-   ##
==========================================
- Coverage   69.65%   69.53%   -0.13%     
==========================================
  Files          93       93              
  Lines        6282     6299      +17     
==========================================
+ Hits         4376     4380       +4     
- Misses       1529     1542      +13     
  Partials      377      377

Impacted Files	Coverage Δ
server/events/yaml/raw/global_cfg.go	`0.00% <0.00%> (ø)`
server/events/pre_workflow_hooks_command_runner.go	`100.00% <100.00%> (ø)`
server/events/yaml/valid/global_cfg.go	`89.61% <100.00%> (+0.27%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 832afea...0d35199. Read the comment docs.

pinkavaj · 2021-02-06T15:10:09Z

server/events/yaml/raw/global_cfg.go

+		withoutSlashes := r.Branch[1 : len(r.Branch)-1]
+		// checked in repo Validate()
+		branchRegex = regexp.MustCompile(withoutSlashes)
+	}


Consider allowing specify single branch using plain branch-name syntax, similar to non-regex for the id.

I considered this. id allows a string or regex, but it feels kinda cumbersome. The default value needs to continue matching all branches. I favored branch field always being a regex (default .*) (and if you want to match a single name, you could just set it to ^master$) so we don't need to condition on which sort of string it is.

If a maintainer prefers the opposite, I'm happy to refactor.

server/events/yaml/valid/global_cfg_test.go

server/events/yaml/raw/global_cfg.go

dghubble · 2021-02-06T22:31:06Z

One reason #1292 wouldn't work in our case, we don't enable the mergeable requirement (we have other Github Apps later in our workflow). We also want Atlantis to completely ignore certain branches, rather than erroring. I think Atlantis being able to filter/ignore PRs for certain branches is independent of Atlantis mergeble/approved concepts.

nishkrishnan

LGTM! Agreed that this implementation is more generic and clean

dghubble · 2021-02-24T18:14:53Z

I'll rebase!

* Add a branch regex to the repo.yaml config to allow Atlantis to accept only pull requests with a given base branch (the branch a PR would merge _into_). * `branch` is optional. By default its unset and Atlantis will match webhooks for pull requests for any branch (no change) Match any PR, ``` repos: - id: /.*/ branch: /.*/ ``` Match only PRs with master or main base branch. ``` repos: - id: /.*/ branch: /(main|master)/ ``` Some repos have special pull request branching practices. For example, plan/apply from feature branches merging into master, but separate flows for merging master into a release branch (where Atlantis isn't used). Adding a regex allows for flexible workflow options. For example, you could have Atlantis ignore "release" branches.

dghubble · 2021-02-24T18:31:24Z

Fixed conflict

Fixes runatlantis#1539 The branch matcher feature has been introduced in runatlantis#1383, but the current implementation was broken and doesn't work at all (runatlantis#1539). If my understanding is correct, there are two problems: (1) The `GlobalCfg` has a default `Repo` instance which always matches any repositries and branches. Therefore the branch matcher never be functional. (2) Validating base branches in `DefaultPreWorkflowHooksCommandRunner.RunPreHooks()` implicitly assumed that users customize `pre_workflow_hooks`, but the assumption isn't always true because it defaults to empty. For (1), I added a new method `MatchingRepo()` to `GlobalCfg` to check `BranchMatches()` for a single `Repo` instance. For (2), I moved validating branch to `DefaultCommandRunner.validateCtxAndComment()`. Since the method has already validated meta data of pull request, I think it's suitable place to check base branches, but please let me know if there is anywhere more suitable.

Fixes #1539 The branch matcher feature has been introduced in #1383, but the current implementation was broken and doesn't work at all (#1539). If my understanding is correct, there are two problems: (1) The `GlobalCfg` has a default `Repo` instance which always matches any repositries and branches. Therefore the branch matcher never be functional. (2) Validating base branches in `DefaultPreWorkflowHooksCommandRunner.RunPreHooks()` implicitly assumed that users customize `pre_workflow_hooks`, but the assumption isn't always true because it defaults to empty. For (1), I added a new method `MatchingRepo()` to `GlobalCfg` to check `BranchMatches()` for a single `Repo` instance. For (2), I moved validating branch to `DefaultCommandRunner.validateCtxAndComment()`. Since the method has already validated meta data of pull request, I think it's suitable place to check base branches, but please let me know if there is anywhere more suitable.

…nfig Fixes runatlantis#1695 The branch matcher feature has been implemented in runatlantis#1383 and runatlantis#1768. There is only an example for it, but not in the reference. https://github.com/runatlantis/atlantis/pull/1383/files#diff-5dd8dd3b7c37191b78109efaaa1bb73184ff7a1690632d687fed7cd748847f5eR31-R34 Add missing the `branch` key in the reference for server side repo config. I also add a warning for `mergeable` requirement to check the `branch` setting because I think a typical branch protection rule only restricts a default branch. We should let users know that someone can potentially bypass it without the `branch` restriction in atlantis.

Fixes runatlantis#1695 The branch matcher feature has been implemented in runatlantis#1383 and runatlantis#1768. There is only an example for it, but not in the reference. https://github.com/runatlantis/atlantis/pull/1383/files#diff-5dd8dd3b7c37191b78109efaaa1bb73184ff7a1690632d687fed7cd748847f5eR31-R34 Add missing the `branch` key in the reference for server side repo config. I also add a warning for `mergeable` requirement to check the `branch` setting because I think a typical branch protection rule only restricts a default branch. We should let users know that someone can potentially bypass it without the `branch` restriction in atlantis.

) Fixes #1695 The branch matcher feature has been implemented in #1383 and #1768. There is only an example for it, but not in the reference. https://github.com/runatlantis/atlantis/pull/1383/files#diff-5dd8dd3b7c37191b78109efaaa1bb73184ff7a1690632d687fed7cd748847f5eR31-R34 Add missing the `branch` key in the reference for server side repo config. I also add a warning for `mergeable` requirement to check the `branch` setting because I think a typical branch protection rule only restricts a default branch. We should let users know that someone can potentially bypass it without the `branch` restriction in atlantis.

Fixes #1539 The branch matcher feature has been introduced in #1383, but the current implementation was broken and doesn't work at all (#1539). If my understanding is correct, there are two problems: (1) The `GlobalCfg` has a default `Repo` instance which always matches any repositries and branches. Therefore the branch matcher never be functional. (2) Validating base branches in `DefaultPreWorkflowHooksCommandRunner.RunPreHooks()` implicitly assumed that users customize `pre_workflow_hooks`, but the assumption isn't always true because it defaults to empty. For (1), I added a new method `MatchingRepo()` to `GlobalCfg` to check `BranchMatches()` for a single `Repo` instance. For (2), I moved validating branch to `DefaultCommandRunner.validateCtxAndComment()`. Since the method has already validated meta data of pull request, I think it's suitable place to check base branches, but please let me know if there is anywhere more suitable.

Fixes runatlantis#1539 The branch matcher feature has been introduced in runatlantis#1383, but the current implementation was broken and doesn't work at all (runatlantis#1539). If my understanding is correct, there are two problems: (1) The `GlobalCfg` has a default `Repo` instance which always matches any repositries and branches. Therefore the branch matcher never be functional. (2) Validating base branches in `DefaultPreWorkflowHooksCommandRunner.RunPreHooks()` implicitly assumed that users customize `pre_workflow_hooks`, but the assumption isn't always true because it defaults to empty. For (1), I added a new method `MatchingRepo()` to `GlobalCfg` to check `BranchMatches()` for a single `Repo` instance. For (2), I moved validating branch to `DefaultCommandRunner.validateCtxAndComment()`. Since the method has already validated meta data of pull request, I think it's suitable place to check base branches, but please let me know if there is anywhere more suitable.

…natlantis#1784) Fixes runatlantis#1695 The branch matcher feature has been implemented in runatlantis#1383 and runatlantis#1768. There is only an example for it, but not in the reference. https://github.com/runatlantis/atlantis/pull/1383/files#diff-5dd8dd3b7c37191b78109efaaa1bb73184ff7a1690632d687fed7cd748847f5eR31-R34 Add missing the `branch` key in the reference for server side repo config. I also add a warning for `mergeable` requirement to check the `branch` setting because I think a typical branch protection rule only restricts a default branch. We should let users know that someone can potentially bypass it without the `branch` restriction in atlantis.

dghubble changed the title ~~Add a branch regex matcher to the server side repo config~~ Add a branch matcher to the server side repo config Feb 2, 2021

pinkavaj reviewed Feb 6, 2021

View reviewed changes

pinkavaj mentioned this pull request Feb 6, 2021

add branch-allowlist repo-config parameter #1292

Closed

nishkrishnan approved these changes Feb 24, 2021

View reviewed changes

nishkrishnan merged commit 00a5bc0 into runatlantis:master Feb 24, 2021

nishkrishnan mentioned this pull request Feb 25, 2021

Is it possible to prevent circumventing the mergeable rule for applying a plan in GitHub? #1407

Closed

enummela mentioned this pull request Apr 8, 2021

Support for repos[*].branch field in server side repo config #1490

Open

ericsonrumuy7 mentioned this pull request Apr 28, 2021

Branch matcher does not work in atlantis v0.17.0 #1539

Closed

vikiuvb mentioned this pull request Jun 8, 2021

Atlantis server side repo config branch filter not working #1623

Open

minamijoyo mentioned this pull request Aug 23, 2021

Validate base branches in DefaultCommandRunner #1768

Merged

minamijoyo mentioned this pull request Sep 2, 2021

docs: add missing the branch key in the reference for server side repo config #1784

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a branch matcher to the server side repo config #1383

Add a branch matcher to the server side repo config #1383

dghubble commented Feb 2, 2021 •

edited

Loading

codecov bot commented Feb 2, 2021 •

edited

Loading

pinkavaj Feb 6, 2021

dghubble Feb 6, 2021 •

edited

Loading

pinkavaj Feb 7, 2021

dghubble commented Feb 6, 2021 •

edited

Loading

nishkrishnan left a comment

dghubble commented Feb 24, 2021

dghubble commented Feb 24, 2021

Add a branch matcher to the server side repo config #1383

Add a branch matcher to the server side repo config #1383

Conversation

dghubble commented Feb 2, 2021 • edited Loading

Why?

codecov bot commented Feb 2, 2021 • edited Loading

Codecov Report

pinkavaj Feb 6, 2021

Choose a reason for hiding this comment

dghubble Feb 6, 2021 • edited Loading

Choose a reason for hiding this comment

pinkavaj Feb 7, 2021

Choose a reason for hiding this comment

dghubble commented Feb 6, 2021 • edited Loading

nishkrishnan left a comment

Choose a reason for hiding this comment

dghubble commented Feb 24, 2021

dghubble commented Feb 24, 2021

dghubble commented Feb 2, 2021 •

edited

Loading

codecov bot commented Feb 2, 2021 •

edited

Loading

dghubble Feb 6, 2021 •

edited

Loading

dghubble commented Feb 6, 2021 •

edited

Loading